Skip to content

Assorted cleanup#50

Merged
behrmann merged 14 commits intovarlink:masterfrom
behrmann:ruff
Feb 23, 2025
Merged

Assorted cleanup#50
behrmann merged 14 commits intovarlink:masterfrom
behrmann:ruff

Conversation

@behrmann
Copy link
Collaborator

The ruff commits in order

  1. the first commit adds a ruff config
  2. the second only runs ruff format and can be recreated from that
  3. the third adds a few trailing commas to make the diff smaller to the current version
  4. adds a ruff format check to CI

The rest is cleanup, mostly around string formatting.

Taken from mkosi, using the common systemd line length.
Copy link
Collaborator

@jelly jelly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this looks good, I'll need to get used to ruff format as I've always used different kinds of quotes at times and I am not a super huge fan of:

self.call_this_thing(
       foo,
       bar,
)

But that doesn't feel bad enough to nack this.

After all these changes we should be able to run ruff check in CI as well now right? Locally I see some failures with:

E722 and maybe more as I didn't check out this branch with extra rules enabled yet. But I'm happy to ignore some specific rules which we can clean up later.

Silence the warning for overly long lines and the warnings for direct comparions
with bools (E712). The latter *cannot* be fixed by using the bare values, as is
recommended and needs investigation in the future.
Bare except catches BaseException, which includes SystemExit and
KeyboardInterrupt. This is probably not intended in the places where this is
used. Instead except Exception instead, which all "normal" exceptions derive
from. Where possible use the correct exception, use exception-free code or
annotate a more likely exception for the future.
@behrmann
Copy link
Collaborator Author

After all these changes we should be able to run ruff check in CI as well now right? Locally I see some failures with:

E722 and maybe more as I didn't check out this branch with extra rules enabled yet. But I'm happy to ignore some specific rules which we can clean up later.

I hadn't done that yet, because those the bare excepts needed a real look at this. I silenced the minor warnings in one commit and fixed the bare commits in another (hopefully), so that we can add the ruff check right away.

and I am not a super huge fan of: [horizontal style]

Yeah, it definitely takes some getting used to, but it has the upside of making diffs much more localized and smaller. That's what sold me on it.

@behrmann
Copy link
Collaborator Author

Lest I forgot the second commit in the series changed, because I also reformatted setup.py

@behrmann behrmann merged commit e593ce0 into varlink:master Feb 23, 2025
8 checks passed
@behrmann behrmann deleted the ruff branch February 23, 2025 12:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants